Skip to content

Add zmq_timers support#657

Merged
gummif merged 2 commits intozeromq:masterfrom
cacharle:add-timers
Apr 23, 2025
Merged

Add zmq_timers support#657
gummif merged 2 commits intozeromq:masterfrom
cacharle:add-timers

Conversation

@cacharle
Copy link
Copy Markdown
Contributor

@cacharle cacharle commented Apr 17, 2025

Fixes #575

This is a draft to add support for the zmq_timers_* functions.

Please let me know what are the requirements to get it merged.

@cacharle cacharle marked this pull request as ready for review April 17, 2025 18:30
gummif
gummif requested changes Apr 18, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@gummif gummif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. What is missing is some minor fixes and some simple tests should be sufficient.

@cacharle
Copy link
Copy Markdown
Contributor Author

cacharle commented Apr 18, 2025

I've had to rename zmq::timers_t to zmq::timers, the first one was making the tests crash (I'm guessing a linker name conflict with libzmq)

Do you have any workaround to keep the _t convention?

@gummif
Copy link
Copy Markdown
Member

gummif commented Apr 18, 2025

I think zmq::timers is fine. Not a fan of the _t convention, also unfortunate we are using the same namespace as libzmq.

@cacharle
Copy link
Copy Markdown
Contributor Author

cacharle commented Apr 18, 2025

Alright then I think this PR is ready

gummif
gummif reviewed Apr 22, 2025
View reviewed changes
gummif
gummif reviewed Apr 22, 2025
View reviewed changes
gummif
gummif reviewed Apr 22, 2025
View reviewed changes
@gummif
Copy link
Copy Markdown
Member

gummif commented Apr 23, 2025

Could you try rebasing your branch on latest master, I just fixed the CI runners.

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 23, 2025

Pull Request Test Coverage Report for Build 14624243775

Details

  • 33 of 39 (84.62%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.07%) to 86.301%
Changes Missing Coverage Covered Lines Changed/Added Lines %
zmq.hpp 33 39 84.62%
Totals Coverage Status
Change from base Build 14615581445: -0.07%
Covered Lines: 882
Relevant Lines: 1022
💛 - Coveralls

@gummif
Copy link
Copy Markdown
Member

gummif commented Apr 23, 2025

Very good. Thanks for this.

@gummif gummif merged commit 34a9b3a into zeromq:master Apr 23, 2025
9 of 10 checks passed
@botantony botantony mentioned this pull request Jun 20, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gummif gummif gummif requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support for zmq timers?

3 participants

@cacharle @gummif @coveralls